Skip to content

Fix bulk_create not setting M2M on FK-related objects#583

Merged
amureki merged 7 commits intomodel-bakers:mainfrom
benaduo:fix/bulk-create-fk-nested-m2m
Mar 26, 2026
Merged

Fix bulk_create not setting M2M on FK-related objects#583
amureki merged 7 commits intomodel-bakers:mainfrom
benaduo:fix/bulk-create-fk-nested-m2m

Conversation

@benaduo
Copy link
Copy Markdown
Contributor

@benaduo benaduo commented Mar 10, 2026

Summary

  • Fixes baker.make() with _bulk_create=True not applying M2M relationships
    when the M2M field is on a FK-related object, specified via double-underscore
    syntax (e.g. home__dogs=[dog])
  • Adds HomeOwner test model and a regression test reproducing the exact
    scenario from the issue

Motivation

When a M2M value is passed as fk_field__m2m_field=[...], baker stores the
value in the sub-baker's m2m_dict during prepare(), but _handle_m2m() is
gated behind if _commit: and never executes in the bulk_create path.
_save_related_objs() only saves FK rows without touching M2M, and the two
existing M2M loops in bulk_create() only scan the top-level model's own
fields — neither handles the fk__m2m kwarg pattern. The result is that
home.dogs.set([dog]) is never called and the M2M relation stays empty.

Changes

  • model_bakery/baker.py: import FieldDoesNotExist from
    django.core.exceptions; add FK-nested M2M handling loop at the end of
    bulk_create() — parses kwargs for fk_field__m2m_field patterns, validates
    field types, guards against custom through models, and calls .set() on the
    FK object for each bulk-created entry
  • tests/generic/models.py: add HomeOwner model with ForeignKey to Home
  • tests/test_baker.py: add test_create_through_foreign_key_field to
    TestCreateM2MWhenBulkCreate

Testing

pytest tests/test_baker.py::TestCreateM2MWhenBulkCreate -v

Author

Benjamin Aduo (@benaduo)

Summary by CodeRabbit

  • New Features

    • Enhanced bulk-create to allow setting many-to-many relations on related objects via a double‑underscore path during bulk creation.
  • Tests

    • Added a regression test verifying related-object M2M targets are populated after bulk creation.
    • Added a small test model to support the regression scenario.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 10, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a second post-processing pass in bulk_create to apply many-to-many relations specified via single-level double-underscore kwargs on foreign-key-related objects (e.g., home__dogs=[...]), with field-type and auto-created through-model checks; adds a HomeOwner model and a regression test (the test currently contains a mis-indentation).

Changes

Cohort / File(s) Summary
Core M2M handling
model_bakery/baker.py
Added a second-pass in bulk_create that parses single-level fk__m2m kwargs, validates the FK is ForeignKey/OneToOneField, validates the target is a ManyToManyField, ignores non-auto-created through models, and bulk-inserts through-table rows linking created FK-related instances to provided M2M objects.
Test models
tests/generic/models.py
Added HomeOwner model with home = models.ForeignKey(Home, on_delete=models.CASCADE) to enable FK→M2M test scenarios.
Regression test
tests/test_baker.py
Added test_create_through_foreign_key_field to verify home__dogs=[dog] is applied for bulk-created HomeOwner entries; the test contains a mis-indentation that will cause a syntax/indentation failure.

Sequence Diagram

sequenceDiagram
    participant User
    participant Baker as baker.bulk_create
    participant ORM as Django ORM
    participant FKObj as FK-related object

    User->>Baker: call bulk_create(entries, home__dogs=[dog], _bulk_create=True)
    Baker->>Baker: prepare entries, persist FK fields
    Baker->>ORM: bulk_create(prepared_entries)
    ORM-->>Baker: return created objects
    Baker->>Baker: parse single-level `fk__m2m` kwargs
    loop per created object
        Baker->>FKObj: getattr(entry, fk_field_name) -> fk_obj
        Baker->>FKObj: insert through-model rows linking fk_obj and provided m2m objects (bulk_create)
        FKObj-->>Baker: through rows persisted
    end
    Baker-->>User: return created entries
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • amureki

Poem

🐰 I hopped through code with nimble paws,
I linked each home to dogs with double-underscore laws,
I checked each field and skipped odd throughs,
Bulk-made friends now saved in twos,
A carrot cheer for passing news! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: fixing bulk_create to properly set M2M relationships on FK-related objects, which is exactly what the PR implements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Member

@amureki amureki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @benaduo !

Thank you for opening this PR and fighting against linters and typing checks. I understand that this experience could be improved and will try to address things there.

Regarding the change, very well done with handling all the different cases and scenarios here, good guardrails!

I noted a couple of things we could improve before moving forward.

Comment thread tests/test_baker.py Outdated
Comment thread model_bakery/baker.py Outdated
Comment thread model_bakery/baker.py Outdated
benaduo and others added 5 commits March 26, 2026 11:52
When baker.make() is called with _bulk_create=True and a M2M field is
specified via double-underscore FK lookup syntax (e.g. home__dogs=[dog]),
the M2M relationship was never applied to the saved FK object.

Root cause: during the prepare() phase, baker correctly stores M2M values
in the sub-baker's m2m_dict, but _handle_m2m() is gated behind `if _commit:`
and therefore never executes in the bulk_create path. Subsequently,
_save_related_objs() only saves FK rows and makes no M2M calls. The two
existing M2M loops in bulk_create() scan only the top-level model's direct
and reverse M2M fields, neither of which covers the fk__m2m kwarg pattern.
The net result is that home.dogs.set([dog]) is never called, leaving the
M2M relation empty in the database.

Fix: after all entries have been bulk-created and FK objects saved, inspect
the kwargs dict for double-underscore patterns of the form fk_field__m2m_field.
For each pattern where the prefix resolves to a ForeignKey or OneToOneField
on the current model and the suffix resolves to a ManyToManyField on the
related model, call .set() on the FK object for every created entry.

Changes:
- model_bakery/baker.py: add FK-nested M2M handling loop at the end of
  bulk_create(), after the existing reverse-relation M2M loop
- tests/generic/models.py: add HomeOwner model with ForeignKey to Home,
  providing the minimal fixture required to reproduce the reported scenario
- tests/test_baker.py: add test_create_through_foreign_key_field to
  TestCreateM2MWhenBulkCreate, reproducing the exact scenario from the issue
  (baker.make(HomeOwner, home__dogs=[dog], _quantity=10, _bulk_create=True))

Fixes model-bakers#490

Author: Benjamin Aduo (@benaduo)
When baker.make() is called with _bulk_create=True and a M2M field is
specified via double-underscore FK lookup syntax (e.g. home__dogs=[dog]),
the M2M relationship was never applied to the saved FK object.

Root cause: during the prepare() phase, baker correctly stores M2M values
in the sub-baker's m2m_dict, but _handle_m2m() is gated behind `if _commit:`
and therefore never executes in the bulk_create path. Subsequently,
_save_related_objs() only saves FK rows and makes no M2M calls. The two
existing M2M loops in bulk_create() scan only the top-level model's direct
and reverse M2M fields, neither of which covers the fk__m2m kwarg pattern.
The net result is that home.dogs.set([dog]) is never called, leaving the
M2M relation empty in the database.

Fix: after all entries have been bulk-created and FK objects saved, inspect
the kwargs dict for double-underscore patterns of the form fk_field__m2m_field.
For each pattern where the prefix resolves to a ForeignKey or OneToOneField
on the current model and the suffix resolves to a ManyToManyField on the
related model, call .set() on the FK object for every created entry.

Only auto-created through models are supported; fields using a custom
through= model are skipped since they require explicit through instance
creation with extra field values baker cannot infer.

FieldDoesNotExist is caught specifically (not bare Exception) so that
unexpected errors still surface rather than being silently swallowed.

Changes:
- model_bakery/baker.py: import FieldDoesNotExist from django.core.exceptions;
  add FK-nested M2M handling loop at the end of bulk_create() with specific
  exception handling and auto-created through model guard
- tests/generic/models.py: add HomeOwner model with ForeignKey to Home,
  providing the minimal fixture required to reproduce the reported scenario
- tests/test_baker.py: add test_create_through_foreign_key_field to
  TestCreateM2MWhenBulkCreate, reproducing the exact scenario from the issue
  (baker.make(HomeOwner, home__dogs=[dog], _quantity=10, _bulk_create=True))

Fixes model-bakers#490

Author: Benjamin Aduo (@benaduo)
I actually like this. Yes lets do this

Co-authored-by: Rust Saiargaliev <hey@amureki.me>
Hmmm... I wanted to be very thorough in my docs ... but yeah I see how this can be harder to read... very nice catch

Co-authored-by: Rust Saiargaliev <hey@amureki.me>
This will indeed lead to less db hits... i agree

Co-authored-by: Rust Saiargaliev <hey@amureki.me>
@benaduo benaduo force-pushed the fix/bulk-create-fk-nested-m2m branch from bd9e8af to 006da06 Compare March 26, 2026 10:53
GitHub's commit suggestion feature preserved incorrect indentation
from the diff view for the comment block in baker.py (2 spaces
instead of 4) and the for loop in test_baker.py (6 spaces instead
of 8).
@benaduo
Copy link
Copy Markdown
Contributor Author

benaduo commented Mar 26, 2026

Hi @amureki, I've addressed all three suggestions:

  • stronger assertion on the test,
  • the bulk_create optimization for the M2M rows,
  • and the cleaner comment.

I also resolved the merge conflict with main after #582 landed.

One thing to flag though: the ruff C901 complexity check is now failing on bulk_create (scoring 19 against the limit of 10). The optimization you suggested pushed it over the threshold. Happy to refactor the function into smaller pieces if that's the preferred approach or follow whatever direction you think is best here.

Thanks for the thorough review!

@amureki
Copy link
Copy Markdown
Member

amureki commented Mar 26, 2026

@benaduo thanks for addressing my comments!
I think, once merged on main, this would be good to go.
I wonder which version to release it under. Probably I'd go with 1.24 to be on the safer side... 🤔

Again, many thanks for taking the time and headspace for all these contributions, which makes me very happy! 🙏

@amureki
Copy link
Copy Markdown
Member

amureki commented Mar 26, 2026

One thing to flag though: the ruff C901 complexity check is now failing on bulk_create (scoring 19 against the limit of 10). The optimization you suggested pushed it over the threshold. Happy to refactor the function into smaller pieces if that's the preferred approach or follow whatever direction you think is best here.

Ah, I see, sorry!
I think, we can land this changeset live (maybe with C901 exception) and see if there are any regressions that we accidentally introduced.
And then we could split the logic a bit. C901 already exists somewhere in other places of the logic; it is very hard with model-bakery to keep up with it. :)

The function exceeds ruff's complexity limit of 10 (scores 19)
after the bulk_create optimization for FK-nested M2M rows was
added per reviewer suggestion. Suppressing for now as agreed
with maintainer; refactoring into smaller helpers can follow.
@benaduo
Copy link
Copy Markdown
Contributor Author

benaduo commented Mar 26, 2026

Ok @amureki, all CI checks are now passing including ruff, black and ty. The PR is ready whenever you are!

@amureki amureki merged commit 2d51d7c into model-bakers:main Mar 26, 2026
45 checks passed
amureki added a commit that referenced this pull request Mar 27, 2026
…any-to-one-rel

* origin/main:
  Bump requests from 2.32.5 to 2.33.0 (#590)
  Fix bulk_create not setting M2M on FK-related objects (#583)
  Fix remaining ty type errors in baker.py and generators.py (#589)
  Add regression test for bulk_create with M2M field using explicit related_name (#582)
  Update uv-build requirement from <0.10.0,>=0.9.26 to >=0.9.26,<0.11.0 (#579)
  Bump black from 25.12.0 to 26.3.1 (#588)
  Bump urllib3 from 2.6.2 to 2.6.3 (#587)
  Bump actions/download-artifact from 7 to 8 (#580)
  Bump actions/upload-artifact from 6 to 7 (#581)
  Bump django from 5.2.9 to 5.2.12 (#585)
  Bump pillow from 12.0.0 to 12.1.1 (#586)
  Fix type annotations (#584)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants